[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mfranczy
Once this PR has been reviewed and has the lgtm label, please assign arangogutierrez for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Welcome @mfranczy!

It looks like this is your first PR to kubernetes-sigs/node-feature-discovery 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/node-feature-discovery has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

Hi @mfranczy. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	5b8206a
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/66dafca7a2f27200084cef62
😎 Deploy Preview	https://deploy-preview-1845--kubernetes-sigs-nfd.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

/cc @ArangoGutierrez

I am in the process of building nfd client prototype. I reduced the scope with commands. ORAS SDK provides a lot of useful functions so there is no need to introduce much for artifact manipulation (thanks ORAS folks!).

Thanks @mfranczy for the proposal. Just thinking aloud below, not proposing anything or having any answers.

It looks like we need to rely on node labels to be able to integrate this with the kubernetes components (scheduler etc). If we want to do without changes to the kubernetes API or extend the kubernetes components with some plugins looking at NFD CRDs (or similar) or something exotic like that.

What we need to think is the node label management on NFD side. We have a bunch of built-in labels (that get populated by just deploying NFD) but a lot of stuff is only visible in NodeFeatures only. For automatizing the creation NodeFeatureRules (a future, step, yes) we need to understand what we get automatically, from the built-in-labels and what we need custom labels for. We could use namespacing e.g. prefix all image compatibility labels with oci.feature.node.kubernetes.io/ (or opencontainers.org/ or similar).

Regarding the expressions, would CEL (or parts of it) be useful there?
https://github.com/google/cel-spec

@marquiz thanks for looking into it.

It looks like we need to rely on node labels to be able to integrate this with the kubernetes components (scheduler etc). If we want to do without changes to the kubernetes API or extend the kubernetes components with some plugins looking at NFD CRDs (or similar) or something exotic like that.

I think looking at NFD CRDs would be a good choice for start.

What we need to think is the node label management on NFD side. We have a bunch of built-in labels (that get populated by just deploying NFD) but a lot of stuff is only visible in NodeFeatures only. For automatizing the creation NodeFeatureRules (a future, step, yes) we need to understand what we get automatically, from the built-in-labels and what we need custom labels for. We could use namespacing e.g. prefix all image compatibility labels with oci.feature.node.kubernetes.io/ (or opencontainers.org/ or similar).

Right, a few missing pieces I already identified, like discovery of runtime configuration (/proc/sys) that could be a built-in source only visible to NodeFeatures. Basically for image compatibility there is a lot of things already covered by the project (like kernel modules, config, version, usb and pci devices etc.). Maybe we don't need to introduce a new namespacing for the image compatibility purpose. From my perspective we could extend the built-in functionality and keep the original NFD labels (or features - if the label is not populated to a node) to describe the compatibility. What do you think?

Regarding the expressions, would CEL (or parts of it) be useful there?
https://github.com/google/cel-spec

Looks very promising, I think we could use it. That's a good idea.

Thanks @mfranczy for the proposal. Just thinking aloud below, not proposing anything or having any answers.

It looks like we need to rely on node labels to be able to integrate this with the kubernetes components (scheduler etc). If we want to do without changes to the kubernetes API or extend the kubernetes components with some plugins looking at NFD CRDs (or similar) or something exotic like that.

What we need to think is the node label management on NFD side. We have a bunch of built-in labels (that get populated by just deploying NFD) but a lot of stuff is only visible in NodeFeatures only.

A node don't know what images(not only one, usually many) will run on it until the image is scheduled to the node, and each image's compatibility may require different set of node features, it is difficult to set node feature rules to label node automatically.

Considering the danymic situation, extending scheduler with plugin to validate the image compatibility and node features seems to be more viable.

Considering the danymic situation, extending scheduler with plugin to validate the image compatibility and node features seems to be more viable.

In that context agree - the scheduler would determine compatibility, but then the container runtime doing the pull would do so based on more fine-grained selection for optional performance. It would be scoped to what the scheduler scoped it to (architecture, GPU and devices, etc).

Regarding the expressions, would CEL (or parts of it) be useful there?
https://github.com/google/cel-spec

Looks very promising, I think we could use it. That's a good idea.

@marquiz I will try to implement this in the prototype and will inform if there is anything that stops us to use CEL.